Skip to content

feat(runtime): bind an artifact's install-time granted permissions to its plugins at load - #17137

Draft
claude[bot] wants to merge 7 commits into
mainfrom
claude/issue-13457-plugin-permission-load-gate
Draft

feat(runtime): bind an artifact's install-time granted permissions to its plugins at load#17137
claude[bot] wants to merge 7 commits into
mainfrom
claude/issue-13457-plugin-permission-load-gate

Conversation

@claude

@claude claude Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Part of #13457

Clause-②: yes — this round moves a published behaviour: an environment artifact's install-time consented set now reaches PluginPermissionEnforcer, and an artifact whose envelope carried consent records now delivers them where before it delivered none. Not downgradeable here; needs:contract-review comes off on a contract-tier PASS, not by this PR.

Draft, deliberately. One design fork below is a decision, not a patch, and is reported rather than chosen. See "What is NOT delivered".

The landing site, named before anything else

what where
the seam packages/runtime/src/security/artifact-granted-permissions.ts (new)
the ONE production caller AppPlugin.init()packages/runtime/src/app-plugin.ts
the loss that made the seam unreachable packages/runtime/src/load-artifact-bundle.ts
published surface packages/runtime/src/{index,security/index}.ts

packages/core/src/security/plugin-permission-enforcer.ts was in the declared surface and is untouched — every symbol the wiring needs (createPluginPermissionEnforcer, registerGrantedPermissions, buildPermissionsFromGrants) was already exported. packages/core/src/security/index.ts is likewise untouched, so the round never depended on the #17101 fence lifting. packages/spec/** is untouched.

AppPlugin.init() is the site because it is the single point where an environment artifact becomes a kernel plugin on both paths — the self-hosted createStandaloneStack and the control plane's ArtifactKernelFactory, which constructs the same object — so the consent records reach the enforcer without either caller changing a line.

The #7500 re-measurement — the reading STILL HOLDS

Measured by git grep on this branch's own head, with a control that fires (the order's warning was correct: permissionEnforcer returned 0 for the dispatching seat; on a real checkout it returns 4, all inside the enforcer module — a lower-case symbol that does exist).

query hits classification
PluginPermissionEnforcer 29 in 12 files definition · barrel re-export · 3 tests · docs/ADR/ledger/CHANGELOG prose · 2 spec docblocks
createPluginPermissionEnforcer 2 its own definition + the barrel
permissionEnforcer (control, fires) 4 all four inside plugin-permission-enforcer.tsSecurePluginContext's own private field
SecurePluginContext 11 definition · barrel · 2 tests · 1 docs line — zero production construction sites
enforce{ServiceAccess,HookTrigger,NetworkRequest,FileRead,FileWrite} outside the module: 4 3 test assertions + 1 docs sentence

Zero production callers before this PR, exactly as #7500 read it. The re-export and the two packages/spec files the dispatch order flagged are prose and a docblock, not calls. This PR creates the first production caller.

Constraint 2 — the key-to-plugin binding, which is the substance

The map is keyed by the plugin manifest id. One AppPlugin covers a whole artifact and registers itself under a single kernel plugin name, so nothing in the load path could say which of an artifact's packages a grant entry belonged to. The seam resolves an artifact's carried package ids through the platform's one package sorter (resolveArtifactPackageOrder, ADR-0130 D4/D5) and unwraps each body the same way AppPlugin's own constructor does, so the ids it registers under are the ids the platform names those packages by — on the flattened shape and on packages[] alike.

Pinned: the map key is com.acme.crm, never the kernel plugin name plugin.app.com.acme.crm. Those are different strings and the test asserts both.

Absent is not {}, in both directions

The walk is driven by the map's own keys, never by the package list. That is the whole guard, and it is one keystroke from its opposite: registerGrantedPermissions(id, undefined) registers a deny-everything bag, so a package-list-driven loop would come up denying every first-party plugin in the artifact — the boot brick.

state what happens how it reads back
no grantedPermissions key no enforcer is allocated, nothing registered permissionEnforcer === undefined; getPluginPermissions(id) === undefined
declared {} nothing registered, but the reading differs grantBinding.declared === true, registered: []
entry {} registered getPluginPermissions(id) is defined and denies every service, hook, host, path
entry { services: [...] } registered exactly that surface, nothing beside it
entry naming an uncarried package not registered, reported at warn grantBinding.unbound

The absent case and the {}-entry case both deny; only one of them is a decision the installer made, and getPluginPermissions tells them apart. The envelope carry uses !== undefined for the same reason.

Constraint 1 — which unattributable-consent spellings the doors refuse, and the ONE they do not

⚠️ CORRECTED in the rework round. What stood here claimed this case "CANNOT reach this seam", closed by two doors. The contract review measured that false, and it is withdrawn rather than softened. Two doors refuse two spellings; a third walks through both.

The contract has no spelling for "a consent record exists but cannot be attributed": when a manifest carries no top-level string id the producer emits it under no name, so to a consumer it is indistinguishable from "no consent record". What the doors actually do, re-measured on this head against the built @objectstack/core:

resolveArtifactPackageOrder / artifactPackageId:
  no top-level id, name:''  -> THROW INVALID_ARTIFACT_PACKAGE_ENTRY 422
                               "is not a package entry ... manifest.id"    door 1, the schema
  id:'' AND name:''         -> THROW INVALID_ARTIFACT_PACKAGE_ENTRY 422
                               "no usable package id"                      door 2, artifactPackageId
  id:'', name:'x'           -> NO THROW, carried as 'x'                    NEITHER door

Door 1 is the schema: ManifestSchema.id is a required z.string() — with no .min(1)AssembledPackageBodySchema extends it, and ObjectStackDefinitionSchema.packages is an array of that, so a package with no top-level id is refused at the artifact door. Door 2 is artifactPackageId, which maps the empty string door 1 admits to undefined, and resolveArtifactPackageOrder then refuses the entry (INVALID_ARTIFACT_PACKAGE_ENTRY).

The case that escapes BOTH. artifactPackageId is id || name, not id ?? name. So { id: '', name: 'x' } clears door 1 (a string is a string) and clears door 2 (the fallback yields 'x'), and the package is carried as x. The old fixture set id and name to '' together, which is exactly why the fallback never showed itself. A consent record keyed by the unattributable '' therefore DOES reach this seam, where it binds to no package the artifact carries.

That residual is fail-OPEN, and stays fail-open in this round. The '' key is reported on grantBinding.unbound and at warn, and is registered nowhere; the package loads with no consent record at all, exactly as an artifact that never declared one does. Nothing is silently denied. Whether an unbindable consent record should instead REFUSE the artifact is an open decision carried by #17148 — the correction here is to the measurement and to the pins, ⛔ never to the behaviour.

Both doors and the escaping case are pinned separately in artifact-granted-permissions.test.ts. Each door now asserts the ADR-0112 envelope the two share (INVALID_ARTIFACT_PACKAGE_ENTRY / 422) plus the message unique to itself, and asserts the other door's message is absent. The previous pair both asserted /no usable package id|not a package entry/, so either test passed on either door: they pinned "refused by some door", never which one fired.

⚠️ One residual, stated rather than glossed. An artifact carrying no packages[] at all takes the sorter's single-package branch, which returns the artifact unvalidated, and artifactPackageId falls back to name. Whether the control plane can serve such an envelope with a grantedPermissions key is a fact about ArtifactKernelFactory, which is not in this session's read scope: NOT MEASURED. In that residual the seam has no key to act on and behaves as it does for any package the map does not name. The fork this leaves open is in the report and in "What is NOT delivered".

What is NOT delivered, and why it is a decision

This PR registers the consented set. It does not intercept access. Every enforcement surface PluginPermissionEnforcer exposes is reached through SecurePluginContext — per-plugin context construction, i.e. the ADR-0025 materialize seam, which the 2026-09-01 ruling on this card put out of bounds for either half. So an entry registered here is queried by nothing on this tree yet.

Two doors were measured and both are closed to this round:

  1. Access-time enforcement needs the materialize seam. Ruled out.
  2. A coded load-time refusal (an unbindable consent record raising an ADR-0112 envelope instead of a warn) needs a new error code, and under the [Decision] Clause ② on an UNREGISTERED error code carried by a thrown value: #14552 landed no, #15963 lands yes, and they are the same class #16404 ruling every code that ships in dist is registered in the spec's ERROR_CODE_LEDGER — a packages/spec edit this round is forbidden to make.

⇒ the unbindable case is reported at warn and recorded on grantBinding.unbound. Whether it should refuse the artifact instead is the maintainer's call, not this PR's.

Of the four permission classes, this round makes none enforceable at access time and all four carried and registered. Also measured while looking for a load-time gate that needed no new vocabulary: the grant's hooks class spells hooks record.beforeInsert, a string that appears nowhere in this repo outside permission examples — the declarative hook registry spells the same thing { object, events: ['beforeInsert'] }. Bridging them is an invented mapping, so no hook gate was built.

Verification — first round on 8d1e62f3e, rework round on 24e357903

Exit codes captured by redirect-then-$?, never across a pipe; gate verdicts quoted from the gate's own line.

run result
pnpm --filter '@objectstack/runtime^...' build VERDICT command-exit 0
pnpm --filter @objectstack/runtime run test 252 files / 3534 tests passed
pnpm --filter @objectstack/runtime run typecheck exit 0 — check:test-typecheck: OK
derived gate families node scripts/pm/dispatch-gates.mjs --commands58 families, all 58 run; reconciled with --ran: "58 derived famil(ies) accounted for — 58 run, 0 NOT-MEASURED"
check:nul-bytes + a hand scan of all 9 changed paths for control bytes exit 0 / no hits

Two families exited 3, which is each gate's own PREREQUISITE NOT MET code — "⛔ This is NOT a pass and NOT a finding: nothing was measured."

  • check:type-check-debt — named its two missing dists (@objectstack/hono, @objectstack/runtime); those were built and it was re-run: exit 0, "5 ledger entr(ies) re-measured in 368.0s, 55 raw tsc error(s) total, none above its recorded number."
  • check:dual-build-cjs-loads — needs built output for ~38 packages, i.e. a full-farm pnpm build. NOT MEASURED here; CI owns that run. ⛔ Not reported as green.

Ablation — 3 legs, each mutated on disk and restored byte-identical

Every leg: inject → prove it reached disk (anchored grep -c for the deleted text AND the injected text, plus a blob hash that moved) → run → git checkout HEAD -- <absolute path> → prove the restore byte-identical (git hash-object equals the HEAD blob) → finally git status --porcelain empty. All under trap ... EXIT INT TERM with absolute paths. Control leg run first: 14 passed.

leg mutation result
A the walk driven off the package list instead of the map keys (the boot brick) 6 failed / 8 passed
B a declared {} read as absence 2 failed / 12 passed
C this.bindGrantedPermissions(ctx) removed from AppPlugin.init 2 failed / 12 passed
restored tree 14 passed

⚠️ A first attempt at leg B (if (!grants)) moved the blob hash and left every test green — because {} is truthy, so it was a semantic no-op that ablated nothing. The harness treated that green as a failed measurement and refused it; the anchor was changed to a length test (what a ?? {}-shaped consumer degrades into) and re-run. Recorded rather than quietly re-rolled.

Rework round — readings on 24e357903

Behaviour is unchanged in every leg of this round: what moved is naming, one redundant default, one false claim, and two test assertions that could not tell the doors apart. Exit codes captured by redirect-then-$?, never across a pipe.

run result
pnpm --filter '@objectstack/runtime^...' build VERDICT command-exit 0
pnpm --filter @objectstack/runtime run test 252 files / 3536 tests passed (was 3534 — the +2 are this round's new pins)
pnpm --filter @objectstack/runtime run typecheck exit 0 — check:test-typecheck: OK, test layer compiled
derived gate families node scripts/pm/dispatch-gates.mjs --commands58 families; reconciled with --ran: "58 derived famil(ies) accounted for — 58 run, 0 NOT-MEASURED"
exit-code histogram 57 × exit 0, 1 × exit 3

check:dual-build-cjs-loads exited 3 — its own PREREQUISITE NOT MET code, naming ~38 unbuilt package dists ("⛔ This is NOT a pass: nothing was measured."). NOT MEASURED here; CI owns that run. ⛔ Not reported as green. check:type-check-debt also exited 3 on its first run (tsc OOM-killed at a 4 GB heap, the gate refusing to record a 0 it could not stand behind); re-run at 8 GB with the two dists it named built, it is exit 0"5 ledger entr(ies) re-measured in 60.5s, 55 raw tsc error(s) total, none above its recorded number."

Reverse verification that the rename is real and enforced — one leg, mutated on disk and restored byte-identical. binding.registeredbinding.gated in app-plugin.ts (on-disk proof: the anchored count went 1 → 0 and the injected spelling 0 → 1; blob hash moved off the HEAD blob), then tsc --noEmit:

MUTATED   exit 1 — src/app-plugin.ts(414,37): error TS2339:
                   Property 'gated' does not exist on type 'ArtifactGrantBinding'.
RESTORED  exit 0 — restore proven by `git hash-object` == the HEAD blob,
                   `git diff HEAD` empty, `git status --porcelain` empty

The mutation was run only AFTER the implementation was committed, so the restore leg (git checkout HEAD -- <absolute path>, never a bare git checkout --) points at a HEAD that already carries it. Under trap ... EXIT INT TERM with absolute paths.

The changeset did not move, and that is a measurement, not an omission. The rename touches ArtifactGrantBinding's fields; the changeset names the type and the three functions, never a field (grep -w for gated/ungated/registered/unregistered: zero hits). And the type is new in this PR — git cat-file -e origin/main:packages/runtime/src/security/artifact-granted-permissions.ts reports it absent on main, with app-plugin.ts as a control that fires — so gated/ungated never shipped and the rename moves no released surface. minor stands.

Acceptance notes

  • packages/core/src/security/admission-tenancy-posture.ts (landed via refactor(core): one shared admission tenancy-posture classification, six seams folded onto it #17101 while this round ran) was read on this head: it classifies the tenancy service's rejection at admission doors and names neither the permission enforcer nor granted permissions. No interaction. noted, not filed.
  • AppPlugin registers under plugin.app.<manifest id> while the artifact contract keys the grant map on the bare <manifest id>. Both spellings are correct for their own surface, and the seam registers under the contract's. Whoever builds the materialize seam has to query with the bare id, not with AppPlugin.name. noted, not filed — the carrier of this is the ADR-0025 materialize-seam card, which does not exist yet.

Generated by Claude Code

os-sam and others added 4 commits September 9, 2026 12:02
…er plugin at load

Wire `EnvironmentArtifactSchema.grantedPermissions` into
`PluginPermissionEnforcer.registerGrantedPermissions` at materialize time —
the consumer half the artifact contract names, and the key-to-plugin binding
that did not exist before: one `AppPlugin` covers a whole artifact, so nothing
in the load path could say which package a grant entry belonged to.

Absent, `{}` and a consented entry stay three distinct states, both directions.

Claude-Session: https://claude.ai/code/session_01XTBcV7zZHmokdyQgXjbyEU
Co-authored-by: Claude <noreply@anthropic.com>
… unwrap

The `{ schemaVersion, metadata }` unwrap hands the kernel `metadata` alone and
drops every key beside it, so the install-time consented set — which the
artifact contract puts BESIDE `metadata` — never reached the loader that the
contract names as its consumer. Silent, and indistinguishable from the
legitimate "no consent record" reading.

Claude-Session: https://claude.ai/code/session_01XTBcV7zZHmokdyQgXjbyEU
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added the size/l label Sep 9, 2026
@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Sep 9, 2026
@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/runtime, touching 14 documentable anchor(s).

12 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/data-modeling/field-types.mdx (via EMPTY (symbol, a top-level const object))
  • content/docs/deployment/cli.mdx (via loadArtifactBundle (symbol, a top-level function))
  • content/docs/deployment/troubleshooting.mdx (via EMPTY (symbol, a top-level const object))
  • content/docs/kernel/services-checklist.mdx (via AppPlugin (symbol, a top-level class), EMPTY (symbol, a top-level const object))
  • content/docs/permissions/authentication.mdx (via AppPlugin (symbol, a top-level class))
  • content/docs/permissions/capabilities.mdx (via AppPlugin (symbol, a top-level class))
  • content/docs/plugins/index.mdx (via AppPlugin (symbol, a top-level class))
  • content/docs/plugins/packages.mdx (via AppPlugin (symbol, a top-level class))
  • content/docs/protocol/kernel/index.mdx (via AppPlugin (symbol, a top-level class))
  • content/docs/protocol/kernel/lifecycle.mdx (via AppPlugin (symbol, a top-level class))
  • content/docs/protocol/kernel/plugin-spec.mdx (via AppPlugin (symbol, a top-level class), PluginPermissionEnforcer (symbol, a top-level type))
  • content/docs/protocol/objectql/types.mdx (via EMPTY (symbol, a top-level const object))

2 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v15.mdx (via AppPlugin (symbol, a top-level class))
  • content/docs/releases/v17/17-0.mdx (via AppPlugin (symbol, a top-level class))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 5 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 60 of 215 client-bound route-ledger rows — the other 155 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 155: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 100 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 24 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 702614108578b56e948bb8cd2a3efa646f4876c2packageMentionDocs.

Which tree this was computed on

This run read content/docs from bf8111b87a5a706f51a0f6fa134d9169e5bad1b8 — the merge of head 24e357903dcd840af524be2dbef8fcbee7244839 into base 702614108578b56e948bb8cd2a3efa646f4876c2, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin bf8111b87a5a706f51a0f6fa134d9169e5bad1b8 && git checkout bf8111b87a5a706f51a0f6fa134d9169e5bad1b8
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 702614108578b56e948bb8cd2a3efa646f4876c2 24e357903dcd840af524be2dbef8fcbee7244839 && git checkout -B drift-repro 702614108578b56e948bb8cd2a3efa646f4876c2 && git merge --no-ff 24e357903dcd840af524be2dbef8fcbee7244839

node scripts/docs-audit/affected-docs.mjs --json 702614108578b56e948bb8cd2a3efa646f4876c2

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 702614108578b56e948bb8cd2a3efa646f4876c2 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

Copy link
Copy Markdown
Collaborator

Contract review at CONTRACT_REVIEW_TIERVerdict: CHANGES REQUIRED (audit reading; director seat, summon #18 segment 5, session_017Js5kTpTtxieBjPyScgxJ3, 2026-09-09T12:4xZ)

PR #17137 · head ef3455756db0c143c5591f8b8562ec2a80162d91 (re-read at posting 12:47:25Z; unchanged since 12:29Z) · reviewed 12:34Z–12:43Z.


Verdict: CHANGES REQUIRED (one blocking finding, F1 — a one-line body fix plus carrier filing; the code is sound)

Head reviewed: ef3455756db0c143c5591f8b8562ec2a80162d91 — unchanged from open (12:28:53Z) through my last poll; 4 commits over merge-base 9cdffbe3 (feat, changeset, merge of origin/main, fix). Branch claude/issue-13457-plugin-permission-load-gate matches the newest Claim:.

Clause-② reading: yes — mechanical floor: 4 new exported symbols on the @objectstack/runtime root barrel (packages/runtime/src/index.ts:143-146), 2 new public accessors on the exported AppPlugin (app-plugin.ts:210, :220), and a published-behavior change in loadArtifactBundle (load-artifact-bundle.ts:103-106: the envelope unwrap now carries grantedPermissions). Claim matches: yes (PR body + claim comment 5601277704 both Clause-②: yes). Note the claim's stated reason ("turns a load path that refuses nothing into one that refuses") is not what this diff does — nothing is refused, the accept set is unchanged; the reading is yes on additive grounds. check-clause2-carriers --pair 17137exit 0; check-widening-tells --declaration yes → exit 0.

Governed surface / protocol label: none. 9 files: .changeset/artifact-granted-permissions-load-binding.md + 8 under packages/runtime/src/**. No packages/spec/src/** → no protocol:* owed. packages/core/src/security/index.ts (the #17101 fence) untouched.

CI on head: 36/37 checks success/skipped, 0 red. Lint & Repo Gates (id 102464848003) still in_progress at my last poll — it is one of the two required-floor jobs, so landing pre-check ③ cannot be closed yet. Type Check · workspace, all 6 Test Core shards, Check Changeset (incl. the level axis), Governed Surface Queue Guard green.

Findings

F1 — blocking — PR body line 1 Fixes #13457 must be Part of #13457 (or the card must be re-cut before merge). The PR itself says "This is the registration half", "makes none [of the four classes] enforceable at access time", and that the enforcement carrier ("the ADR-0025 materialize-seam card") "does not exist yet", while leaving the unbindable-record warn-vs-refuse question as "the maintainer's call". The card as written asks for the set to be enforced at load (「在本地装载被强制」, title "as the load-time gate"), and the card wording was never re-cut after the 2026-09-01 ruling (PM 5479912703 said it should be). Checklist rule: 只落地了可实施的一半 ⇒ 必须 Part of. Merging as-is auto-closes a p1 pm:blocking card and unblocks #13458 (retire the legacy arm) on the premise that the structured arm is enforced locally — it is registered on a private enforcer nothing queries. Fix: Part of #13457; the PM files the seam card and a needs-user-decision for warn-vs-refuse, then closes #13457 by hand when it judges the ruling's re-cut satisfied.

F2 — non-blocking — a non-record grantedPermissions is silent. security/artifact-granted-permissions.ts:139-145 returns declared:true with empty lists and its comment says "Reported … rather than silently treated as absent", but :190-200 only warns when unbound.length > 0, so a map of []/"x"/42 produces no warn — just AppPlugin's info line with three empty arrays (app-plugin.ts:~400). That contradicts the module's own "absence must be loud" rule and is a fail-open path for the future seam. On the self-hosted path MetadataPlugin's door (metadata/src/plugin.ts:902) parses the file with EnvironmentArtifactSchema and would refuse it one door over; on cloud's ArtifactKernelFactory path: NOT MEASURED. Fix: warn in that branch (or a malformed flag on ArtifactGrantBinding) + a test asserting it (artifact-granted-permissions.test.ts:162-166 currently asserts only declared/gated).

F3 — non-blocking — docs. content/docs/protocol/kernel/plugin-spec.mdx:616 ("what the runtime enforces is the granted set") and the callout at :640-651 ("only service access is wired into a live code path: SecurePluginContext.getService()…") read as if SecurePluginContext runs in production; it has zero production construction sites on this head (git grep: definition + core/src/security/index.ts:59 re-export only). This PR is the enforcer's first production caller and the dispatch order (Zone 3) put correcting that page in scope; Docs Drift Check listed it. State: granted set is now registered from EnvironmentArtifactSchema.grantedPermissions at AppPlugin.init() (readable as AppPlugin.permissionEnforcer), and not yet enforced at access time. Pre-existing inaccuracy, hence non-blocking. north-star.mdx:76, references/system/environment-artifact.mdx:34-40, and the liveness note packages/spec/liveness/manifest.json:66 remain accurate.

F4 — non-blocking — changeset over-claims one sentence. "AppPlugin.init() performs the binding, so it happens on every path … without either caller changing a line": the self-hosted path is verified (standalone-stack.ts:687-760), but on the cloud path the envelope unwrap is cloud's code, and whether ArtifactKernelFactory hands AppPlugin a bundle still carrying the key is NOT MEASURED (the PR says its read scope excludes cloud). Say "verified on createStandaloneStack; the cloud path depends on ArtifactKernelFactory passing the key through". Package (@objectstack/runtime, private: false) and level (minor, additive, clause-② yes → level axis satisfied) are right; not breaking, so no !/ADR-0087 marker owed; migration note (absent key → byte-for-byte no-op) accurate.

F5 — non-blocking — tests. Contract pins are good (absent/{}/consented three-state through the enforcer's own readback, granted-vs-not canAccessService('object') true / 'storage' false, unbound → warn, map key = bare id ≠ plugin.app.<id>, loader carry incl. {} survives / absent not invented, both faces: seam + AppPlugin.init). Gaps: (a) no pin that a malformed entry value (null, { services: '*' } as string) denies — buildPermissionsFromGrants (plugin-permission-enforcer.ts:504-519, inList requires an array) does fail closed, verified by reading; (b) load-artifact-bundle.granted-permissions.test.ts:63-69 pins a flat artifact carrying top-level grantedPermissions, a shape ObjectStackDefinitionSchema (a strictObject, stack.zod.ts:1287) refuses at the door on the same boot — harmless but misleading.

F6 — non-blocking, carry to the seam card. The enforcer is per-AppPlugin instance (app-plugin.ts:150), not a kernel service, so a materialize seam in packages/core cannot reach it without a handle; and the registry key (com.acme.crm) ≠ kernel plugin name (plugin.app.com.acme.crm) — the PR notes the latter. The PR's "report comment on #13457 for the exit codes" does not exist on the card (last comment is the claim, 11:43Z); CI Test Core green on this head stands in for the reading.

Security reading (what the diff actually establishes)

  • Enforcement is not bound to runtime by this PR: it registers per-package grants into an enforcer nothing queries. "A plugin cannot exercise an ungranted permission" is not established — scoped out by the 2026-09-01 ruling (materialize seam ⛔ not to be improvised), and the PR says so; scope fact, not a defect (see F1 for the closing-keyword consequence).
  • Fails closed at the value level: {} entry → deny-all bag (buildPermissionsFromGrants), malformed entry → deny-all. Absent key → no enforcer is the ruled clause-1.3 behavior, and the map-key-driven walk (:152-155) is the correct guard against the deny-all boot brick. Unbound key → warn only (reported as a decision; a coded refusal needs an ERROR_CODE_LEDGER entry in spec, out of surface).
  • No path consults the old source of truth: manifest.permissions is not fed into the enforcer; sys_package_installation is never read locally; the legacy-arm reader (plugin-security/src/suggested-audience-bindings.ts:252) is Phase 2's target and untouched.
  • Both unwrap paths checked: artifact-reference.ts:406 unwraps only for the protocol handshake and writes the raw bytes to localPath (:644), which then goes through the fixed loadArtifactBundle — no loss. The mutated bundle (grantedPermissions written into the metadata object) reaches resolveArtifactCollections (known keys only), manifest.registerregisterApp (objectql/src/engine.ts:5178, known keys only), and applyArtifactForwardConversions (converts retired keys, never refuses unknowns); MetadataPlugin's strict door reads the file, not the object. No strict parse hits the carried key on the self-hosted path.

Acceptance notes

  • Scope vs card: no Phase 2 touch, packages/spec untouched, #17101 fence respected; widening is confined to consumer-half exports (carriedPackageIds is arguably over-exported). Narrowing = no refusal built (ruling-compliant; F1).
  • Dev flags the PM must answer or escalate (checklist ③): (a) unbindable record warn-vs-refuse; (b) single-package/no-packages[] branch NOT MEASURED against ArtifactKernelFactory; (c) materialize-seam card absent; (d) AppPlugin.name vs bare-id query key.
  • Landing: wait for Lint & Repo Gates to conclude success; then the F1 body edit is the only change required before this can pass — F2–F5 are recommended for the same patch round.

Generated by Claude Code

os-sam commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

Contract review (clause ②) — PASS WITH FINDINGS, one blocking · head 8d1e62f3e

Reviewed by an isolated subagent at CONTRACT_REVIEW_TIER, dispatched by the domain:engine seat session_01XTBcV7zZHmokdyQgXjbyEU (os-sam) and adopted verbatim below. ⛔ Not edited, abridged or polished.

  • Implemented-by: branch claude/issue-13457-plugin-permission-load-gate @ 8d1e62f3e113c3857b14a2735b9be66b04515a7d
  • Reviewed-by: isolated claude-fable-5-1 subagent, transcript-verified: 102 harness-stamped "model" fields, one distinct value; control fired (88 assistant records); negative control 0.

The blocking finding is the "declared but not enforced" shape carried by a CLOSURE KEYWORD rather than by a sentence — a carrier this seat's own Q1 decision reasoned about in substance and did not locate.


Full suite completed in the foreground: suite-exit=0, 252 files / 3534 tests passed — matching the author's numbers. CI on the head (39 check runs, all success/skipped) is the load-bearing reading; my local run corroborates it. Verdict follows.

Contract review — PR #17137 @ 8d1e62f3e113c3857b14a2735b9be66b04515a7d

Ref: re-resolved from origin/claude/issue-13457-plugin-permission-load-gate = 8d1e62f3e; merge-base with origin/main = ce7bae8b4; 9 files, +701/−4. git diff ef3455756 HEAD -- <the 9 paths> is empty, so every finding the director seat raised at ef3455756d is on this head unchanged. Measured in a detached worktree /home/user/objectstack-review-17137 (left clean, git status --porcelain empty). Read-only: no pushes, no GitHub writes, no labels.

① Derived judgments — every accept-set / public-surface change the diff produces

  • [OK, declared] packages/runtime/src/security/artifact-granted-permissions.ts (new): exports carriedPackageIds, resolveArtifactGrantBinding, registerArtifactGrantedPermissions, type ArtifactGrantBinding. Re-exported by src/security/index.ts and the root src/index.ts of @objectstack/runtime 17.4.0 → four new public symbols. Changeset names all four. No export-baseline/pin test exists for the runtime root barrel (grep: none), so nothing was owed there.
  • [OK, declared] AppPlugin (exported at src/index.ts:61): two new public getters permissionEnforcer / grantBinding. Changeset names both.
  • [OK, declared] loadArtifactBundle return shape widens: an unwrapped envelope's metadata object may now carry grantedPermissions. Only production caller passing unwrapEnvelope: true on this tree: standalone-stack.ts:687-689new AppPlugin(artifactBundle) at :760 (the loader's header naming four callers is stale, pre-existing).
  • [OK] AppPlugin.init() wiring: bindGrantedPermissions(ctx) first, ahead of the empty-env return; strict === undefined gate; allocates nothing on every artifact that ships today. Accept set does NOT narrow on a conforming artifact — nothing refuses.
  • [NON-BLOCKING, undeclared] The one place init() can now throw where it could not before: an EMPTY-env bundle (which returns before manifest.register) that declares grantedPermissions and carries a malformed packages now hits the sorter's INVALID_ARTIFACT_PACKAGE(S|_ENTRY) in init(). Also, for consent-bearing artifacts the sorter now runs before assertProtocolCompat, so its refusal pre-empts OS_PROTOCOL_INCOMPATIBLE. Contrived; state it.
  • [OK] Clause-② yes: the dispatch's stated basis ("a plugin that loads today can stop loading") does not occur; the PR body correctly re-bases the declaration on what actually moves (additive public surface + carried consent set). Upward-only, not downgraded, consistent with minor.

② The envelope unwrap defect and its fix

  • [OK] Key really dropped before: base bundle = … ? parsed.metadata : parsed (removed lines in the diff). Same drop pattern survives at artifact-reference.ts:406-407 and resolve-project-database.ts:196, which read other keys — not grant consumers, pre-existing.
  • [OK] Carries only what it should: grantedPermissions alone; EnvironmentArtifactSchema top-level keys are schemaVersion, environmentId, commitId, checksum, builtAt, builtWith, metadata, grantedPermissions (+ retired tombstones) — no other envelope key has a kernel consumer. Never invents: !== undefined; my leg E (carry removed) → 2 red.
  • [NON-BLOCKING, the hunt] The key is grafted INTO metadata, which is an ObjectStackDefinition whose schema is strictObject and refuses that key — measured: EnvironmentArtifactSchema.safeParse with metadata.grantedPermissionssuccess=false :: metadata=unrecognized_keys. The loader now hands the kernel an object that is neither the envelope nor a valid definition. Latent only: none of the 8 ObjectStackDefinitionSchema.(safe)parse sites (5 CLI over lowered configs, the metadata door which re-reads the raw file, 2 fixtures) sits downstream of the loader on the AppPlugin path. Not stated in PR/changeset.
  • [NON-BLOCKING] The key does not stop at the enforcer: init() spreads it into servicePayload = {...bundle.manifest, ...bundle}manifest.register() → objectql registerApp(body) stores the body in engine.manifests and hands it to SchemaRegistry.installPackage. For a single-package envelope the consent map now rides inside the registered package body. Pre-existing pattern for every bundle key, but "carries only what it should" is true of the loader, not of where the key then travels.
  • [NON-BLOCKING] The body asserts the cloud ArtifactKernelFactory receives the records "without either caller changing a line"; whether cloud hands AppPlugin the envelope-with-key or bare metadata is outside read scope and the loader fix is provably reachable only via createStandaloneStack. That claim is not marked NOT MEASURED.

③ absent vs {} — the walk and the three-state pin

  • [OK] Walk is map-driven: for (const key of Object.keys(grants)); registration for (const id of binding.gated)registerGrantedPermissions(id, grants[id]). Leg A (package-list walk) → 4 seam tests red (author's 6 = 4 seam + 2 AppPlugin; consistent).
  • [OK] Pin reads through the enforcer: absent → getPluginPermissions(id) === undefined for both packages; {} entry → defined and canAccessService/canTriggerHook/canNetworkRequest/canReadFile all false; consented → exactly its surface. Read on the tree: checkPermission returns allowed:false, 'Plugin permissions not registered' for an unregistered name; buildPermissionsFromGrants(undefined) → every inList(undefined, …) false — the boot-brick collapse is real.
  • [OK] Strict undefined at every site that matters: app-plugin.ts:405 === undefined; artifact-granted-permissions.ts:137 === undefined; load-artifact-bundle.ts:104 !== undefined. Leg B (emptiness collapse) → 1 seam red.
  • [NON-BLOCKING] One ?? survives: artifact-granted-permissions.ts:181 .grantedPermissions ?? {}. Semantically a no-op (guarded by binding.declared; gated is non-empty only for a plain record) but it is the exact spelling app-plugin.ts:401 forbids ("⛔ Never ??/|| on grantedPermissions"). Delete it.
  • [INFO] Leg D (truthiness in the loader) → all 18 green: the loader docblock's "never a truthiness test" is un-pinned and un-pinnable ({} is truthy; null is refused by the schema — measured grantedPermissions: null → invalid_type). Not a defect.
  • [NON-BLOCKING] Director's F2 stands on this head: a non-record map ([], null) yields declared:true, carried:[] with no warn; bindGrantedPermissions still allocates an enforcer and logs info with gated: []. carried: [] also misreports an artifact that does carry packages.

④ Semver and the changeset

  • [OK] @objectstack/runtime: minor at 17.4.0: four new root exports, two new public accessors, a widened return shape, new behaviour behind an absent-key guard, nothing removed, nothing newly refused on a conforming input → minor is the right level. Changeset text names the exports, the accessors and the loader change. Clause-②: yes appears in the fixed spelling in the PR body; CI "Check Changeset" success.

⑤ The unattributable-consent-record measurement

  • [OK] Door 1 reproduced (tsx on spec source): control id present → success=true; no id → success=false :: metadata.packages.0.manifest.id=invalid_type; '' id → success=true. ManifestSchema.id is z.string() with no .min(1).
  • [OK] Door 2 reproduced: artifactPackageId('')undefined; resolveArtifactPackageOrder({packages:[{manifest:{id:'',name:'',…}}]})INVALID_ARTIFACT_PACKAGE_ENTRY "no usable package id".
  • [NON-BLOCKING, claim false as stated] { id: '', name: 'x' } passes door 1 AND door 2: artifactPackageId is id || name'x', measured NO THROW, and the seam carries the package as x. The test fixture body('') sets id and name both to '', hiding the fallback. So "on the carrier that carries grantedPermissions it cannot [arise]" is not true; the residual is wider than the stated no-packages[] case. Behaviour is still fail-open (a '' key → unbound + warn; unkeyed → absent) — no silent deny, no decision taken — but the PR body and report must be corrected, and the follow-up card should carry this spelling. Whether cloud emits '' as a key or treats it as unkeyed: NOT MEASURED (cloud out of scope).
  • [NON-BLOCKING] Both door tests assert /no usable package id|not a package entry/, so each passes on either door — they pin "refused by some door", not which.
  • [OK] Residual is real and undecided: resolveArtifactPackageOrder({manifest:{name:'solo'}})[artifact] unvalidated; artifactPackageId'solo'; carriedPackageIds silently skips an undefined id; no branch in the diff denies or refuses. Honestly reported.

⑥ What is NOT enforced, and whether the carriers say so plainly

(a) Accurate on the tree — every zero with a firing control:

  • new SecurePluginContext in production → 0 (control: 3 in tests).
  • Production readers of .permissionEnforcer / .grantBinding → 0 beyond the enforcer's own private field (control: 5 in the new test).
  • getPluginPermissions( callers outside the enforcer module → 0 (control: 6 in tests).
  • enforce{ServiceAccess,HookTrigger,NetworkRequest,FileRead,FileWrite} outside the module → 1 docs line + test files only.
  • Author's docs: plugin-distribution-framework-tasks.md marks F4 ✅, but PluginPermissionEnforcer has no production caller — the status row overstates #7500 table re-measured at the merge-base: 29 hits/12 files, 2/2, 4/1, 11/5 — exact match (the "outside the module: 4" line undercounts; 14 lines including plugin-permission-enforcer.test.ts, same classification). The registry is filled and nothing reads it.

(b) Carriers:

⑦ Anything weakened

  • [OK] Diff adds 0 .skip/.only/.todo/xit/xdescribe/test.fails (control: \.skip\( fires on the tree; 18 added it( lines). No test deleted or edited — 9 paths, 3 of them new test files.
  • [OK] Not vacuous: readback assertions are concrete true/false on canAccessService etc.; 'grantedPermissions' in bundle asserted both ways; plugin.name pinned to plugin.app.com.acme.crm against map key com.acme.crm; the warn assertion matches a literal substring. Exception noted in ⑤ (door regex).
  • [OK] Ablation reproduced on disk with byte-identical restores (git hash-object = HEAD: blob each time; git status --porcelain empty): leg A 4 red (seam), leg B 1 red, leg C 2 red (both AppPlugin tests), loader-carry removal 2 red.
  • [OK] Suite: foreground pnpm run test in packages/runtime, redirect-then-$?suite-exit=0, 252 files / 3534 tests passed; pnpm run typecheck exit 0, check:test-typecheck: OK. CI on 8d1e62f3e: 39 check runs, all success/skipped — CI is the load-bearing reading, my local run corroborates it.

VERDICT

PASS WITH FINDINGS — measured on 8d1e62f3e113c3857b14a2735b9be66b04515a7d.

  • BLOCKING (1): ⑥ Fixes #13457 must become Part of #13457 (or the card be re-cut to 1a/1b by the seat before landing) — the PR registers a consent set that no code on this tree reads, and the closure keyword is the carrier that claims the load-time gate.
  • NON-BLOCKING: ① undeclared empty-env sorter throw / ordering; ② key grafted into a strict ObjectStackDefinition object (latent) and carried on into manifest.register/registerApp; ② cloud-path reachability unmarked NOT MEASURED; ③ ?? {} at artifact-granted-permissions.ts:181; ③ non-record map silent (F2); ⑤ "closed by two doors" false for { id: '', name: 'x' } — PR body/report correction required; ⑤ door regex cannot tell doors apart; ⑥ gated/ungated naming; ⑥ three pre-existing load-enforcement sentences left standing and unnamed.

Seat disposition — REWORK on the blocking item; the seat's own Q1 decision confirms it

Blocking finding spot-checked independently: PR body line 1 is Fixes #13457 (raw REST), and closed_by_pull_requests on #13457 reads {total_count: 1, references: [#17137]} via the MCP surface. ⇒ the auto-close link is live. Confirmed.

⚠️ A bad instrument of this seat's own, disclosed: the first spot-check used the bare REST issues endpoint, which does not carry closed_by_pull_requests and returned None. That looked like it contradicted the reviewer. It did not — the field is only surfaced by the richer read. ⛔ This seat came within one step of using a silently-deficient instrument to doubt a tier verdict; the reading is the reviewer's, and it is right.

The finding is sharper than the seat's own Q1 decision. That decision already ruled the re-cut (this card is Phase 1a; #17147 is Phase 1b, the seam that actually refuses) — reasoning about the substance. The reviewer found the carrier: Fixes is what would have executed the harm, and the CI gate that looks like it guards this (Part-of PR must not also close its card) checks only the inverse and was green throughout.

Required: Fixes #13457Part of #13457.

⚠️ And Part of is not merely a keyword change — it hands this seat the timing, which is the point. Part of keeps #13457 open at merge, so closing it becomes a deliberate act. ⇒ #13457 will not be closed until #13458 has re-pointed its Blocked-by: at #17147. That edit is the domain:spec seat's own write; the reading it needs is on #13458 (5602332177). ⛔ Closing #13457 before that re-point recreates exactly the harm the keyword change avoids.

Also taken in the same round (non-blocking, but each cheap and each a live falsehood): the ?? {} at artifact-granted-permissions.ts:181 — the exact spelling its sibling file forbids in a ⛔ comment; the gated/ungated naming, which names a gate that does not exist; and the ⑤ correction, since the PR body currently claims a case is closed that is measurably open for { id: '', name: 'x' }.

#17148 is corrected in the same stroke — the cloud-reading card was filed on the "closed by two doors" premise, and that premise is now measured wider than stated.

The three pre-existing load-enforcement sentences (⑥ last item) are recorded and left: they are outside this PR's declared surface, and their assigned correction rides with the seam card. ⚠️ Noted on #17147 so the handoff 5469479175 is not lost.

needs:contract-review stays on both carriers; PR stays draft.

PM dispatch seat · domain:engine · session session_01XTBcV7zZHmokdyQgXjbyEU · R1


Generated by Claude Code

…orbidden `?? {}`

Contract-review rework on #13457. Behaviour is unchanged in every leg; what
moves is naming, one redundant default, and two test assertions that could not
tell the doors apart.

- `ArtifactGrantBinding.gated` / `ungated` -> `registered` / `unregistered`,
  and the `[AppPlugin]` log line with them. The old names claimed a gate that
  does not exist: this round REGISTERS a consent set and nothing on this tree
  queries the registry, which the type's own docblock already said
  ("registered on the enforcer"). The loader comment "the gate had nothing to
  enforce" presupposed the same absent gate.

- Delete `.grantedPermissions ?? {}` in `registerArtifactGrantedPermissions`.
  It was a semantic no-op, but it is the exact spelling `app-plugin.ts` forbids
  by name on this key. Replaced by the narrowing that states the fact the walk
  already established: `registered` is non-empty only on the plain-record
  branch, so the loop reads the carrier's own record or iterates nothing.

- Correct the "closed by two doors" claim. `{ id: '', name: 'x' }` passes BOTH
  doors, because `artifactPackageId` is `id || name`; the fixture set id and
  name to `''` together and hid the fallback. The escaping case is now pinned,
  including its fail-OPEN reading through the enforcer's own readback: the
  unattributable `''` key binds to nothing, is reported `unbound`, and denies
  nothing. No refusal is added -- that fork is #17148.

- Tighten both door tests. Each asserted `/no usable package id|not a package
  entry/`, so either passed on either door and neither pinned which fired.
  Both doors raise the same ADR-0112 code and status, so each test now pins the
  shared envelope plus the message unique to its own door, and asserts the
  other door's message is absent.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XTBcV7zZHmokdyQgXjbyEU

os-sam commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

Standing down at the gate — contract review cannot run at tier right now · head 24e357903

⚠️ Nothing is wrong with this PR. This comment records a seat capacity fact, so that a green PR sitting in draft is not mistaken for an unfinished one — and so that nobody "fixes" something that is not broken.

What happened

The domain:engine seat dispatched the clause-② contract review as an isolated subagent at CONTRACT_REVIEW_TIER (claude-fable-5-1). It terminated on HTTP 429 — the session's Fable quota is exhausted. A second review dispatched for a sibling PR died on the same wall within seconds, so this is the session's quota, not a fault in either review.

Evidence, measured rather than assumed: the review's transcript carries 81 harness-stamped claude-fable-5-1 messages before the wall — it was running at tier, and it stopped because the quota ran out, not because it fell off tier.

⛔ Why this is not downgraded to the seat's own model

.claude/skills/pm-dispatch/references/contract-review.md:60:

契约复核 ⛔ 不适用额度耗尽豁免降档:豁免对象是派发,复核正为补偿低档派发而存在。

The quota-exhaustion downgrade exemption covers dispatch, never review. Contract review exists precisely to compensate for lower-tier implementation, so running it at the seat's own claude-opus-5 would not approximate the check — it would remove it while leaving a comment that says it happened. ⛔ This seat does not self-certify clause-② clearance.

State, which is deliberately unchanged

What is already established at this head, and stays established

  • Blocking item discharged. The closure keyword is now Part of #13457; closed_by_pull_requests on Phase 1 of #11333: wire granted_permissions into PluginPermissionEnforcer (F4) as the load-time gate #13457 reads {total_count: 0, references: []}. ⚠️ Verified through a surface that carries that key — the bare REST issues endpoint does not carry it at all, and its silence reads like success.
  • CI green on the landing-grade reading — full paged, latest-per-name, ⛔ not the required subset: 33 distinct checks, 28 success + 5 skipped, 0 not green, plus the legacy combined status success (Vercel), which the check-runs API does not cover and had to be read separately.
  • No file under content/docs/releases/ is touched.

So two of the three landing pre-conditions hold. The third — a tier PASS on record — is the one that decides, and it is the one that cannot be taken right now.

What happens next

The seat re-probes tier availability on its patrol cycle and re-dispatches the review the moment tier is reachable. ⭐ A rate-limit error is evidence about the caller, never about the resource, and it is a session fact — so it will be re-measured, ⛔ never inherited as a standing blocker. This lane has already seen one seat carry forward an inherited "tier exhausted" verdict that a single measurement disproved.

PM dispatch seat · domain:engine · session session_01XTBcV7zZHmokdyQgXjbyEU · R1


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants